Added Error Logging #164
Added Error Logging #164
Conversation
meta-cla
bot
added
the
CLA Signed
There was a problem hiding this comment.
Thanks for the PR!
This is in the right direction, however, we have a colleague whose using this code to run experiments (@shaahins please take a look), so I want to be careful about touching this code.
Generally I have two points of feedback!
For this toy agent, there is a distinction between 1) Errors in the llm output (mostly we can't pull a kernel from it) and 2) our api provider doing something weird like rate limiting. When putting feedback back into the llm we only care about the 1st error class (so Agent Error should only cover the first case) as the 2nd is not actionable by the agent. (ie. Claude can't do anything about us not paying our anthropic bill lol). It is somewhat useful to have an error class for 2 for when we run experiments, but it is not necessary, so I wouldn't include it in this PR.
The second point of feedback is there is a lot of redundancy in your code. Generally, if we can't pull a kernel out of the code (in this iteration of the agent), that's enough to say "ohh something is wrong here".
|
Also @ParamThakkar123 run |
|
Sure @PaliC . I am using pytest to for testing. And all I noted all your feedbacks and suggestion. Will make sure all code changes are aligned with your feedbacks and I would all of them work. Thank you so much! |
|
@PaliC I incorporated all your suggestions in this code and pushed it. Ran the formatter and tested it with pytest. |
…nto AgentErrors
| import os | ||
| from typing import Callable, Dict | ||
|
|
||
| from BackendBench.agent_errors import AgentError |
There was a problem hiding this comment.
please revert changes to the kernel_agent.py. I'm not really sure of the status of this / how it interacts with KernelFalcon.
|
Also apologies for this, but there was another major refactor to this code. I'd recommend rebasing! |
|
Sure @PaliC |
|
@PaliC All changes noted and implemented. Can you please review ? |
There was a problem hiding this comment.
Sorry for the late review (currently traveling).
You'd want to adapt to the FeedbackInfo change (it's a struct and not a dict now.) You can just add AgentError as part of the struct similar to CompilerError.
Also if please do not categorize things as an AgentError if we cannot access the llm (ie. rate limits, bad connection, etc.)
| ) | ||
| response_data = response.json() | ||
| content = response_data.get("output", "") | ||
| if not content or "rate limit" in content.lower(): |
There was a problem hiding this comment.
As discussed before these should not be agent errors as these are issues with connecting to the server.
|
@PaliC Updated the error logging functionality as suggested |
|
@PaliC Made the changes |
…nto AgentErrors
Fixes #74